Remove unused imported types for generated typescript.#18
Remove unused imported types for generated typescript.#18SethCopeland wants to merge 3 commits intoFacilityApi:masterfrom
Conversation
| code.WriteLine(); | ||
| WriteJsDoc(code, service); | ||
| typeNames.Add($"I{capModuleName}"); | ||
| requestTypeNames.Add($"I{capModuleName}"); |
There was a problem hiding this comment.
This isn't technically a request type, but wherever I needed the request types I also needed this so I included it here.
| var typeNames = new List<string>(); | ||
| var requestTypeNames = new List<string>(); | ||
| var responseTypeNames = new List<string>(); | ||
| var dtoTypeNames = new List<string>(); |
There was a problem hiding this comment.
I broke up the types and made this before I realized this wasn't actually being used anywhere.
| if (TypeScript) | ||
| { | ||
| WriteImports(code, typeNames, $"./{Uncapitalize(moduleName)}Types"); | ||
| WriteImports(code, requestTypeNames.Concat(responseTypeNames).ToList(), $"./{Uncapitalize(moduleName)}Types"); |
There was a problem hiding this comment.
I'm assuming the order in which we import things is arbitrary, and we don't care that all the requests are imported first and then response, as opposed to what it was previously import { IRequestOne, IResponseOne, ...IRequestN, IResponseN }
There was a problem hiding this comment.
Maybe we should sort them. Maybe WriteImports should sort them.
There was a problem hiding this comment.
I sorted them alphabetically which maybe is better. Though maybe even better would be to sort them by where they would be used in the file. Which would probably more closely correspond to where they are specified in the fsd file is my guess.
There was a problem hiding this comment.
I could do something like this and replace all this code
private static void WriteImports(CodeWriter code, IReadOnlyList<string> imports, string from, string fileMinusImports)
{
var neededImports = imports.Where(x => fileMinusImports.Contains(x)).ToList();
if (neededImports.Count != 0)
code.WriteLine($"import {{ {string.Join(", ", neededImports.OrderBy(x => fileMinusImports.IndexOf(x)))} }} from '{from}';");
}
though I'd actually have to use a regular expression instead of Contains and IndexOf to ensure IFoo doesn't match IFooBar and I'd have to create a separate StringWriter to get fileMinusImports, which all seems worse.
|
Is this to satisfy some lint rule? I don’t object to the change. I’m not able to really test it until next week. Please run the CodeGen target to update the example service test files. |
|
Many of the tsconfig's specify no unused locals. So it is fixing in some sense a possible compiler error. |
3 participants
We are importing all the types from generated types file when we only need to import a subset of them. While all of them might be used, they are used implicitly in some cases, so this PR is meant to ensure we only import the types that we use explicitly.
For the client this means only including
IServiceName, theIXyzRequests and theIXyzResponses types, and excluding the dtos defined with thedatakeyword in the fsd file.For the server this means only including the
IServiceNameand theIXyzRequest.